Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding blank page for fallback #1441

Merged
merged 10 commits into from
Nov 6, 2020
Merged

Adding blank page for fallback #1441

merged 10 commits into from
Nov 6, 2020

Conversation

prateekbh
Copy link
Member

@prateekbh prateekbh commented Sep 30, 2020

What kind of change does this PR introduce?

Creates a non SSRed html file for caching in service worker. All failed requests would go to this page and the app would render from here

Did you add tests for your changes?

Yes

Summary
Currently we pre-cache all the index.html files of the application. Given the size of the app, these can be many. In order to avoid this we will now create a non ssred version of homepage and upon error will use that to bootstrap the app.

Does this PR introduce a breaking change?
No

@prateekbh prateekbh requested a review from a team as a code owner September 30, 2020 16:12
@changeset-bot
Copy link

changeset-bot bot commented Sep 30, 2020

⚠️ No Changeset found

Latest commit: 9518918

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@developit
Copy link
Member

@prateekbh this is great. One thing I was thinking: it would be amazing if we could check for the fallback URL (or something like 200.html?) in prerender-urls. If the developer has listed it there, we could allow prerendering of that URL rather than using the fully blank page. That way URL misses would still get an App She'll to boot up from.

@prateekbh
Copy link
Member Author

Yes that sounds great! I'll make those changes

@prateekbh prateekbh changed the title [WIP] Adding blank page for fallback Adding blank page for fallback Oct 8, 2020
@prateekbh
Copy link
Member Author

@developit done 👍

@@ -154,7 +171,11 @@ class PrerenderDataExtractPlugin {
this.data_ = JSON.stringify(cliData.preRenderData || {});
}
apply(compiler) {
compiler.hooks.emit.tap('PrerenderDataExtractPlugin', compilation => {
compiler.hooks.emit.tap('PrerenderDataExtractPlugin', (compilation) => {
if (this.location_ === `${PREACT_FALLBACK_URL}/`) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if we still need this with the custom 200 option.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this makes sure that the fallback page has no pre-render data.
Thus in this case the pre-render data will always rely on the current URL .

Co-authored-by: Leah <github.leah@hrmny.sh>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants